a11y(styles): make the Grid selected-card border opaque - #260
Conversation
Pixnop
left a comment
Pixnop
left a comment
There was a problem hiding this comment.
Reviewed this on its own terms rather than as a continuation of #251, and recomputed the alpha table independently before reading yours.
The number
It reproduces exactly. Over the real stack, which is the shell at /70, the grid panel's before:bg-zinc-950/40, then the card's own bg-vsd/50 fill:
| Border alpha | Over a white image | Over a black image | Worst |
|---|---|---|---|
| 25% | 1.53:1 | 1.59:1 | 1.53:1 |
| 50% | 2.32:1 | 2.67:1 | 2.32:1 |
| 75% | 3.41:1 | 4.29:1 | 3.41:1 |
| 100% | 4.82:1 | 6.54:1 | 4.82:1 |
So 4.82:1 worst case against a 3:1 floor.
The backdrop choice is the part worth defending and you got it right. Measuring the border against the panel alone would have given 4.85:1 and looked perfectly reasonable, but background-clip defaults to border-box, so the fill really does sit under the border area, and bg-vsd/50 being translucent means the panel reads through it. Composited properly, the fill over the panel lands on #423323 over a white image and #2b1c0d over a black one. Those are the colours the border is genuinely adjacent to.
Worth noting that 75% would also have cleared the floor at 3.41:1. Going to full opacity rather than the cheapest passing value is the better call, since it leaves headroom if the fill alpha or the panel scrim ever moves, and the test reads the fill alpha out of the source so it will tell the truth if they do.
Colour as the only cue
This is what I came in most sceptical about, since the change is nothing but an alpha. Selected and unselected still differ only in fill and border colour, with no icon, badge or label separating them. Held to the same bar we applied to the links in #251, that looks like a 1.4.1 problem rather than just the 1.4.11 one you filed for.
It is not, and the numbers say why. 1.4.1 is about information carried by hue. Here the selected and unselected borders sit 6.04:1 apart over a white image and 7.57:1 apart over a black one, which is a lightness difference and survives a greyscale render intact. Before this change they were 1.92:1 and 1.84:1 apart, faint enough to be doing almost no work. And the unselected border barely exists in the first place: border-zinc-400/5 reads 1.08:1 against its own fill. So the real cue after this change is bright line against no line, not warm against grey, which is a stronger position than the PR claims for itself. This closes a 1.4.1 gap as well as the 1.4.11 one.
Hover does not collide with any of it. hover:shadow-none applies to both states and moves the shadow, not the border, so the two cues stay independent.
Coherence with #251
No clash. Both borders resolve to the same --color-vsl, now #d49754, so the Settings tile and the mod card show one accent rather than two competing warm tones. That was the thing I would have pushed back on and it is not here.
The two fixes are also more coherent than the diff makes them look. They diverge in method, width on one and opacity on the other, for the reason you spell out: one paints over an arbitrary thumbnail with no computable backdrop and the other over a stack that is fully known. That is a real distinction, not a rationalisation.
The test row
It bites, and in both directions. Reverting the border to border-vsl/25 fails the anchor, as you reported. I also wanted to know whether the ratio assertion is doing anything or whether the regex is carrying it, so I dropped --color-vsl to the vs value and the row failed with Grid selected-card border reads 1.76:1 on the worse extreme, below 3:1. It is a real measurement, not a token-presence check wearing a ratio's clothes.
Reading fillAlpha out of Grid.tsx instead of hardcoding 50 is the right instinct. One thing to keep in mind: the border alpha is still hardcoded to 1 in the test, which is safe only because the anchor forces the bare border-vsl string. Those two have to move together if anyone reintroduces an alpha there. Not asking for a change, just flagging the coupling.
One for a follow-up, not for this PR
The installed state on a mod card is still invisible to assistive tech. ModListCard.tsx passes selected={installed} into GridItem, and GridItem renders a motion.li with no aria-selected, no aria-pressed and no text anywhere saying the mod is installed. A screen reader user gets nothing at all. Your own BackgroundTile in ConfigPage.tsx already handles this correctly with aria-pressed={selected}, so the precedent is sitting in the neighbouring PR.
That is 1.3.1 and 4.1.2 rather than 1.4.11, and clearly out of scope here. Worth its own issue.
Gates
Run on 55618fa.
npm run typecheck: passes.npm run lint:ci: 0 errors, 15 warnings, the same 15 that are already ondev.npm run format:check: clean.npm run test:coverage: 128 files, 1507 passed, 2 skipped. Statements 92.38, branches 89.32, functions 91.3, lines 93.97, all above the floors invitest.config.ts. Matches your figures.
Merge order
Approving. It cannot land ahead of #251 in any case, and I have asked for one change over there (a link anchor in text-contrast.test.ts that cannot fail), so expect a rebase once that settles. The retarget note in your description covers the rest.
55618fa to
ac2ed5f
Compare
|
Rebased onto PR #251's new head after its second review round. The only conflict was the ConfigPage comment block in I also corrected the description and the commit message. Both said the ConfigPage border had no fixed backdrop to measure a ratio against, which #251's review established is wrong on its outer edge: that edge sits on the same fixed section-panel stack as the accent links, at 4.85:1, and that is what holds 1.4.11 there while the Gates on the rebased head, local: typecheck clean, lint 0 errors and the same 15 warnings, format clean, Still a draft, still stacked: #251 first, then this retargets to |
|
Re-checked after the new commit, and the approval stands. The change since Both anchors still bite. Putting the Worth noting for anyone reading later that reverting Gates green locally on |
Fixes #258. The selected-card border in Grid.tsx (bg-vsd/50 border-vsl/25) sat at 25% alpha next to its own dark fill, reading about 1.53:1 worst case, below the 3:1 WCAG 1.4.11 floor for a UI boundary that is the sole indicator of a state (ModListCard.tsx's selected={installed}, the only consumer). Raised during review of PR #251, filed separately since the gap predates it. Unlike ConfigPage's selected background-tile border, whose inner edge paints over an arbitrary user-chosen thumbnail so no single ratio can be asserted for it (its outer edge holds 1.4.11 against the fixed section panel at 4.85:1, and the border-2 there answers 1.4.1 instead), the Grid card's backdrop is fully known on both sides: the card's own bg-vsd/50 fill composited over the grid panel's fixed scrim stack. That makes the ratio actually computable, so the fix is opacity, not width: the border drops its /25 and reads at full strength, which the same compositing math the rest of text-contrast.test.ts already uses puts at about 4.82:1, comfortably clear of the floor. tests/text-contrast.test.ts gets a real assertion where there was previously only a comment explaining why the border went unmeasured. Verified it actually pins the fix: reverting the border to border-vsl/25 makes the test's anchor regex fail to match, since it now requires the bare border-vsl string with no alpha suffix.
ac2ed5f to
9348e33
Compare
|
Rebased onto On the post-approval commit, since it predates the rebase and is worth stating plainly: The conflict was not a content clash. This branch was stacked on #251's branch, so when that merged as a squash and its branch was deleted, this one still carried the four pre-squash commits whose content Both sides of the shared contrast test survived, which was the thing worth checking. All eight of #251's link anchors are still there with the tightened Re-ran the two mutations against the rebased head and both still bite. Putting the Gates green on Approval stands. Merging. |
2 participants
Summary
The selected-card border in
Grid.tsxreads below the 3:1 WCAG 1.4.11 floor for a UI boundary that carries a state on its own. This makes it opaque, which the same worst-case compositing the rest oftests/text-contrast.test.tsuses puts at about 4.82:1, and adds a real assertion where there was previously only a comment explaining why the border went unmeasured.Observed problem
Raised during review of PR #251, filed separately as #258 since the gap predates that PR: "The Grid selected card border reads 1.53:1 worst case at 25 percent alpha over
bg-vsd/50, up from 1.34, and the test declines to assert on it. I agree with not asserting and the comment explaining why is honest, but the selected state there is carried by fill and border colour with nothing else, so it is a 1.4.11 gap that predates you and deserves its own issue rather than a comment in a test."Grid.tsx'sGridItem, the only place this border ships:selectedis set byModListCard.tsxasselected={installed}, the only consumer: an installed mod's card in the ModDB browse grid gets this border and fill, with nothing else marking it as selected.Root cause
border-vsl/25sits at 25% alpha next to the card's ownbg-vsd/50fill. Since the defaultbackground-clip: border-boxpaints the fill under the border area too, the border's effective backdrop is that fill, itself composited over the grid panel's own scrim stack (GridWrapper'sbg-zinc-950/40, itself under the page shell'sbg-zinc-950/70). Verified the exact ratio with a standalone script reproducingtext-contrast.test.ts's own compositing math (over,luminance,contrast) against the real token values:--color-vslis#d49754,--color-vsdis#4f3110.Fix
Unlike
ConfigPage.tsx's selected background-tile border, whose inner edge paints over an arbitrary user-chosen thumbnail so no single ratio can be pinned for it (its outer edge sits on the fixed section panel at 4.85:1, which is what holds 1.4.11 there, and theborder-2answers 1.4.1 instead), this card's backdrop is fully known on both sides: the card's ownbg-vsd/50fill composited over the grid panel's scrim stack. That makes the ratio actually computable, so the fix here is opacity, not width:border-vsl/25becomes plainborder-vsl, full alpha. Width is unchanged.Regression proof
tests/text-contrast.test.tspreviously only pinned that this border tracked--color-vsl, with a comment explaining why no ratio was asserted. It now computes the real worst-case ratio (reading the fill's alpha straight out ofGrid.tsxrather than hardcoding it, so the assertion stays accurate if that changes) and asserts it clears the 3:1 non-text floor.Verified the assertion actually pins the fix, not just the presence of a token: reverted the border to
border-vsl/25and confirmed the test's anchor regex fails to match (it requires the bareborder-vslstring with no alpha suffix), then restored the fix and reran clean.Testing
npm run typecheck: passes.npm run lint:ci: 0 errors, 15 pre-existing warnings.npm run format:check: passes.npm run test:coverage: 128 files, 1,507 passed, 2 skipped, 0 failed. Coverage 92.38% statements, 89.32% branches, 91.3% functions, 93.97% lines, all at or above thevitest.config.tsfloor.npm run build:unpack: passes on Linux x64.Base branch
This PR targets
fix/issue-248-vsl-link-palette(PR #251), notdev. The lighter--color-vsltoken, thetext-contrast.test.tscompositing infrastructure (themeColor,assertReadable, the "brand accent" describe block), and the ConfigPage border precedent this fix follows all live on that branch and not yet ondev. This should be retargeted todev(or rebased and reopened against it) once #251 merges; until then it is a stack, not an independent change. Rebased onto #251's second review round on 2026-08-27.Limitations
Not verified visually in the running app, for the same reason as #251: no safe, non-disruptive way to drive it through the affected screen on this machine. This rests on the same compositing math the rest of the file uses, not an eyeballed comparison.
Related issues
Fixes #258.